SDK, Runtime: Recover JSON-RPC frames containing unpaired UTF-16 surrogates - #2283

Why

This PR fixes a problem that appeared in the github-app but appears to be a broader fix in the SDK.

Fixes github/app#678
Fixes github/app#1055

...and likely others.

The symptom is that the model picker in the App is permanently empty for some users, along with the quota counter and the agents/skills lists. All four report request cancelled, the failure is 100% reproducible across restarts, and re-authorization does not clear it.

Root cause appears to be a single unpaired UTF-16 surrogate in a CLI response (github/app#1055 has good data):

• serde_json correctly rejects lone surrogates per RFC 8259, failing with unexpected end of hex escape.
• The JSON-RPC read loop treats any decode failure as fatal: it breaks, then drains pending_requests, so every in-flight call resolves as RequestCancelled.
• Consumers that multiplex several RPCs over one client lose all of them at once. In the desktop app a single shared client carries models.list, account.getQuota, agents.discover and skills.discover, so one bad frame takes out all four.
• is_transport_failure() is true, so the consumer spawns a fresh CLI, which refetches the same payload and dies at the same byte offset. Deterministic, and it survives restarts — matching the reporter's "same column 256886 across every restart".

The CLI subprocess is the victim here, not the cause: its write EPIPE flood is just the SDK having closed the pipe after the parse error.

What changed

Repair unpaired surrogates to U+FFFD (matching JavaScript's String.prototype.toWellFormed) and retry the decode once.

Two properties keep this contained:

• The repair only runs after a strict parse has already failed. Well-formed frames never enter the repair path, so they are byte-identical to before and the happy path allocates nothing.
• Frames the repair cannot fix stay fatal, exactly as before. Without a trustworthy decode we cannot tell which pending request a frame belonged to, and silently dropping it would hang that caller forever — send_request_with_inline_callback awaits its oneshot with no transport-layer deadline. Those still return the original serde_json error, preserving the existing fast-fail-and-replace recovery.

Net effect: exactly one input class changes behavior — a frame whose only defect is an unpaired surrogate.

The scanner tracks string context and consumes non-\u escapes whole, so a literal C:\\ud83d is never mistaken for an escape prefix, and valid surrogate pairs pass through untouched.

Demo

rust/tests/jsonrpc_test.rs covers this end of it:

• the root-cause payload produces the exact error string from the issue logs
• a #1055-shaped frame is recovered and delivered with U+FFFD substituted, and the connection stays up for a subsequent request
• an unrepairable frame remains fatal — this guards the narrow scope above and fails if a caller is ever left hanging
• valid surrogate pairs and escaped backslashes are untouched, verified in a frame that also contains a lone surrogate so the repair path genuinely runs

Reverting the fix while keeping the tests fails lone_surrogate_frame_is_recovered_without_closing_connection and valid_pairs_and_escaped_backslashes_are_untouched, both unwrapping Protocol(RequestCancelled).

The fix was also validated against the real desktop app and real CLI subprocesses before being brought upstream, by splicing a lone surrogate into live models.list frames via a temporary debug-only injector (never committed).

Same injected payload, two builds:

With the fix, a single CLI pid survived all three corrupted frames, across two different RPCs on the shared channel:

WARN jsonrpc_read_loop: recovered JSON-RPC frame containing unpaired UTF-16 surrogates
     error=unexpected end of hex escape at line 1 column 72  length=25422

The original serde_json message is preserved in the error= field, so the diagnostic is kept rather than swallowed.

Notes

• Review focus: repair_lone_surrogates. It is a hand-rolled byte scanner on the JSON-RPC read path, so the string-context and backslash-parity handling is where a bug would hide. It only ever runs on already-failed frames.
• This is client-side hardening. The real fix belongs with whatever produces the lone surrogate — it should not reach the wire — but the SDK should not lose an entire CLI connection over one bad character. The producer path is not in this repo, so its origin could not be established here.
• Adjacent issue found while investigating, deliberately not addressed here: send_request_with_inline_callback has no timeout, so any lost response hangs its caller indefinitely. That is the reason unrepairable frames must stay fatal, and it is worth revisiting on its own.
• Other language SDKs are out of scope. JavaScript's JSON.parse accepts lone surrogates natively so the TS SDK likely never had this bug; the strict-parser SDKs should be assessed separately rather than receiving unverified cross-language changes.
• github-app vendors this crate, so the fix reaches the desktop app via scripts/sync-copilot-sdk.sh once this merges.